chore: iaw uses org setting insted of feature flag #396
Conversation
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) ✅ code/snyk check is complete. No issues have been found. (View Details) |
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) ✅ code/snyk check is complete. No issues have been found. (View Details) |
| if !config.GetBool(configuration.FF_IAW_ENABLED) { | ||
| return nil, cli.NewFeatureUnderDevelopmentError("") | ||
| if !config.GetBool(configIgnoreApprovalEnabled) { | ||
| return nil, snyk_errors.Error{ |
There was a problem hiding this comment.
I'll change this to an error coming from the Error Catalog before merging.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| @@ -247,6 +248,28 @@ func (a *snykApiClient) GetSastSettings(orgId string) (*sast_contract.SastRespon | |||
| return &response, err | |||
| } | |||
|
|
|||
| func (a *snykApiClient) GetOrgSettings(orgId string) (*contract.OrgSettingsResponse, error) { | |||
| endpoint := fmt.Sprintf("%s/v1/org/%s/settings", a.url, url.QueryEscape(orgId)) | |||
| res, err := a.client.Get(endpoint) | |||
There was a problem hiding this comment.
suggestion: use clientGet() instead
There was a problem hiding this comment.
Added it, only think I don't especially like is the fact that clientGet is for versioned API's and the one we are using is v1, but I'm guessing it should be fine?
CatalinSnyk
force-pushed
the
chore/CLI-1006_iaw_use_org_setting
branch
from
5db2d8a to
49856e6
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| @@ -0,0 +1,16 @@ | |||
| package contract | |||
|
|
|||
| type OrgIgnoreSettings struct { | |||
There was a problem hiding this comment.
nitpick: please link to the API spec this relates to
| @@ -43,6 +44,32 @@ func addCreateIgnoreDefaultConfigurationValues(invocationCtx workflow.Invocation | |||
| }) | |||
| } | |||
|
|
|||
| func getOrgIgnoreApprovalEnabled(engine workflow.Engine) configuration.DefaultValueFunction { | |||
There was a problem hiding this comment.
Suggestion: please add a test
CatalinSnyk
force-pushed
the
chore/CLI-1006_iaw_use_org_setting
branch
from
0d97852 to
b5fd0d9
Compare
| } | ||
|
|
||
| return &response, err | ||
| } |
There was a problem hiding this comment.
Bug: Incorrect Endpoint and Missing Status Code Handling
The GetOrgSettings function uses the incorrect endpoint /v1/org/{orgId}/settings instead of /v1/orgs/{orgId}/settings, which will cause 404 errors in production. Additionally, it fails to check HTTP status codes. For any non-200 response (e.g., 404, 401, 403) with a JSON body, json.Unmarshal succeeds into a zero-valued OrgSettingsResponse. This causes callers to misinterpret API, authentication, or network failures as the Ignore Approval Workflow being "disabled," silently blocking the feature with a misleading message. A status code check (e.g., for 200 OK) should be added, or clientGet used.
Previously, the IAW create workflow was gated behind a feature flag check. For EA, customers should be able to opt in/out of using the ignore approval, so a new org setting was added for them to toggle on/off.
This PR just swaps out the feature flag check with an fetching the org settings and checking whether IAW is enabled for the current org before proceeding with the ignore create workflow.
Ref: CLI-1006